Skip to content

fix(api-server): handle Keycloak service-account- prefix in OIDC username matching#1465

Merged
mergify[bot] merged 1 commit intomainfrom
fix/keycloak-service-account-prefix
Apr 24, 2026
Merged

fix(api-server): handle Keycloak service-account- prefix in OIDC username matching#1465
mergify[bot] merged 1 commit intomainfrom
fix/keycloak-service-account-prefix

Conversation

@markturansky
Copy link
Copy Markdown
Contributor

@markturansky markturansky commented Apr 24, 2026

Summary

  • Keycloak client credentials tokens set preferred_username to service-account-<clientId> (e.g. service-account-ocm-ams-service), not the raw client ID (ocm-ams-service)
  • GRPC_SERVICE_ACCOUNT is populated from the clientId secret key, so the comparison username == serviceAccountUsername always failed
  • Adds isServiceAccount() helper that matches both the raw client ID and the Keycloak-prefixed form
  • Includes unit tests for the new helper

Evidence

Decoded the actual OIDC token issued by Red Hat SSO via client credentials grant:

{
  "sub": "90d50f80-2bb6-4b7a-8781-9e322691dc33",
  "preferred_username": "service-account-ocm-ams-service",
  "azp": "ocm-ams-service"
}

This confirms the service-account- prefix is added by Keycloak for all client credentials tokens.

Root cause trace

  1. Runner calls CP token server → gets raw OIDC access token
  2. Runner sends token to api-server gRPC → pre-auth interceptor extracts preferred_username = "service-account-ocm-ams-service"
  3. Interceptor compares to GRPC_SERVICE_ACCOUNT = "ocm-ams-service"no match → CallerTypeService not set
  4. Framework auth interceptor runs → sets username = "service-account-ocm-ams-service"
  5. WatchSessionMessages checks IsServiceCaller(ctx) → false
  6. Falls through to ownership check: "service-account-ocm-ams-service" \!= "mturansk"PERMISSION_DENIED

Test plan

  • Unit tests pass (TestIsServiceAccount — 6 cases)
  • Merge and auto-deploy to Stage
  • Verify api-server logs show CallerTypeService being set
  • Verify runner WatchSessionMessages no longer returns PERMISSION_DENIED

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes

    • Enhanced service account caller attribution by implementing improved validation logic that provides more reliable authentication handling and better support for various account configuration formats and username extraction scenarios.
  • Tests

    • Added extensive test coverage for service account validation, comprehensively validating multiple authentication scenarios including exact account matches, formatted variations, and edge cases involving empty configurations or partial matches.

…name matching

Keycloak client credentials tokens have preferred_username set to
"service-account-<clientId>" rather than the raw client ID. The
GRPC_SERVICE_ACCOUNT env var is populated from the clientId secret key
(e.g. "ocm-ams-service"), but the JWT username is
"service-account-ocm-ams-service". The direct comparison always failed,
so CallerTypeService was never set and WatchSessionMessages returned
PERMISSION_DENIED.

Add isServiceAccount() helper that matches both the raw client ID and
the Keycloak-prefixed form.

Evidence: decoded the OIDC token from Red Hat SSO client credentials
grant — preferred_username="service-account-ocm-ams-service".

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
@netlify
Copy link
Copy Markdown

netlify Bot commented Apr 24, 2026

Deploy Preview for cheerful-kitten-f556a0 canceled.

Name Link
🔨 Latest commit 70440c4
🔍 Latest deploy log https://app.netlify.com/projects/cheerful-kitten-f556a0/deploys/69ebfcaad8b9690008d1c41b

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 24, 2026

📝 Walkthrough

Walkthrough

Updated gRPC bearer-token interceptors to delegate service-account caller attribution to a new isServiceAccount helper function. The helper validates usernames against both raw and Keycloak-prefixed (service-account-) forms of the configured service account. Added corresponding unit tests for the helper.

Changes

Cohort / File(s) Summary
Bearer Token Interceptors
components/ambient-api-server/pkg/middleware/bearer_token_grpc.go
Refactored unary and stream interceptors to use isServiceAccount helper for service-account detection. Helper returns false if configured account is empty, otherwise matches raw username or Keycloak-prefixed form (service-account-{account}). Sets CallerTypeService only when helper returns true.
Bearer Token Tests
components/ambient-api-server/pkg/middleware/bearer_token_test.go
Added TestIsServiceAccount unit test covering exact matches, Keycloak-prefixed matches, and edge cases (empty username, empty configured account, partial matches).

Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (1 error, 1 warning)

Check name Status Explanation Resolution
Security And Secret Handling ❌ Error PR grants privileged CallerTypeService access based solely on JWT username pattern matching without validating the azp (OAuth2 client identity) claim. Extract azp claim from JWT and validate both username AND client identity in isServiceAccount() to bind authorization to intended OAuth2 client.
Docstring Coverage ⚠️ Warning Docstring coverage is 14.29% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (6 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title follows Conventional Commits format (fix(scope): description) and accurately describes the main change: handling Keycloak service-account- prefix in OIDC username matching.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Performance And Algorithmic Complexity ✅ Passed isServiceAccount performs O(1) string comparisons called once per gRPC request with no loops, N+1 patterns, or expensive operations.
Kubernetes Resource Safety ✅ Passed PR modifies Go authentication middleware only; no Kubernetes manifests, RBAC policies, or infrastructure-as-code present. Custom check for Kubernetes resource safety is not applicable.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/keycloak-service-account-prefix
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch fix/keycloak-service-account-prefix

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@components/ambient-api-server/pkg/middleware/bearer_token_grpc.go`:
- Around line 98-104: isServiceAccount currently treats a JWT as a service
account solely by preferred_username pattern; change its signature to accept and
validate the client identity (azp/client_id) as well and require that azp
matches the configuredAccount (or a configured service client id) in addition to
the username check. Update the two call sites that invoke isServiceAccount (the
handlers that extract preferred_username) to also extract the azp/client_id
claim from the parsed JWT and pass it into the revised isServiceAccount
signature (adjust any variable names such as jwtUsername and configuredAccount
to include jwtAzp/jwtClientID), and ensure the logic enforces both username
pattern OR explicit client id match before returning true.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: bcee3422-603d-4ad0-bf87-6dba4e996baa

📥 Commits

Reviewing files that changed from the base of the PR and between 954024d and 70440c4.

📒 Files selected for processing (2)
  • components/ambient-api-server/pkg/middleware/bearer_token_grpc.go
  • components/ambient-api-server/pkg/middleware/bearer_token_test.go

Comment on lines +98 to +104
func isServiceAccount(jwtUsername, configuredAccount string) bool {
if configuredAccount == "" {
return false
}
return jwtUsername == configuredAccount ||
jwtUsername == keycloakServiceAccountPrefix+configuredAccount
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Service caller attribution is tied only to username text, which is too weak for authz.

At Line 102-Line 103, isServiceAccount grants service caller status from preferred_username pattern alone. Since CallerTypeService gates privileged paths (e.g., inbox stream access), this should also bind to a client identity claim (e.g., azp/client_id) rather than username shape only.

Proposed hardening
-func isServiceAccount(jwtUsername, configuredAccount string) bool {
+func isServiceAccount(jwtUsername, jwtAZP, configuredAccount string) bool {
 	if configuredAccount == "" {
 		return false
 	}
+	if jwtAZP != configuredAccount {
+		return false
+	}
 	return jwtUsername == configuredAccount ||
 		jwtUsername == keycloakServiceAccountPrefix+configuredAccount
}

Then pass azp extracted from JWT at the two call sites (Line 32 and Line 59).

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@components/ambient-api-server/pkg/middleware/bearer_token_grpc.go` around
lines 98 - 104, isServiceAccount currently treats a JWT as a service account
solely by preferred_username pattern; change its signature to accept and
validate the client identity (azp/client_id) as well and require that azp
matches the configuredAccount (or a configured service client id) in addition to
the username check. Update the two call sites that invoke isServiceAccount (the
handlers that extract preferred_username) to also extract the azp/client_id
claim from the parsed JWT and pass it into the revised isServiceAccount
signature (adjust any variable names such as jwtUsername and configuredAccount
to include jwtAzp/jwtClientID), and ensure the logic enforces both username
pattern OR explicit client id match before returning true.

@mergify mergify Bot added the queued label Apr 24, 2026
@mergify
Copy link
Copy Markdown
Contributor

mergify Bot commented Apr 24, 2026

Merge Queue Status

  • Entered queue2026-04-24 23:38 UTC · Rule: default
  • Checks skipped · PR is already up-to-date
  • Merged2026-04-24 23:38 UTC · at 70440c4dfe73a0568de6c71572e26571aa8428fd · squash

This pull request spent 9 seconds in the queue, including 1 second running CI.

Required conditions to merge

@mergify mergify Bot merged commit 89f4a70 into main Apr 24, 2026
57 checks passed
@mergify mergify Bot deleted the fix/keycloak-service-account-prefix branch April 24, 2026 23:38
@mergify mergify Bot removed the queued label Apr 24, 2026
@markturansky markturansky restored the fix/keycloak-service-account-prefix branch April 27, 2026 16:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant